New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

QUIC-1590-widget-markdown-card #279

Merged

allanpope merged 40 commits into master from QUIC-1590-widget-markdown-card

Jul 28, 2022

Contributor

johnniedfeldt commented Jul 18, 2022

Description

Add support for MarkdownCard

💁‍

Looks like this

🤨

PR check-list

Add Jira ticket number to the PR title or to the commit if the PR only has one commit.
Your final merge commit message is in the format <type>: <description> as per conventional commits.
- Example: fix: add shadow to image QUIC-123

johnniedfeldt and others added 11 commits

July 13, 2022 10:16


          Initial Markdown Card

3a7443a


          Merge branch 'master' into QUIC-1590-widget-markdown-card

0b2f05e


          Update story controls

3f29696


          Fix StoryBook MarkdownCard Basic.

fadd8eb


          fix: resolve ts error

50cfd26


          Basic Markdown Card Styling

cc49dc6


          Markdown styling changes. Added different heading text sizes based on…

a0c858a

… heading. Added internal and external links to MarkdownCard.


          Updated <hr> rendering for markdowncard

f151578


          Heading styling update. Sub lists styled. Checkboxes implemented.

2bf292c


          Updated structure to make it easier to add Markdown. There is some is…

23ff243

…sue with empty lines/space around lists and sublists not rendering as anticipated.


          Minor adjustments

3a806e8

johnniedfeldt requested a review from allanpope

July 18, 2022 21:44

Contributor Author

johnniedfeldt commented Jul 18, 2022 •

edited

Loading

Jest unittests having issues with ReactMarkdown

allanpope reviewed

View reviewed changes

Contributor

allanpope left a comment

Nice one, looking pretty good. Just a few comments, yell out if you disagree or have any questions

src/components/ContentCards/ContentCards.tsx Outdated Show resolved Hide resolved

src/components/Heading/Heading.tsx Outdated Show resolved Hide resolved

src/components/MarkdownCard/examples/cheatsheet.md Outdated Show resolved Hide resolved

src/components/MarkdownCard/MarkdownCard.test.tsx Show resolved Hide resolved

src/components/MarkdownCard/MarkdownCard.tsx Outdated Show resolved Hide resolved

src/components/Heading/Heading.tsx Outdated Show resolved Hide resolved

src/components/MarkdownCard/MarkdownCard.stories.tsx Show resolved Hide resolved

src/components/MarkdownCard/MarkdownCard.stories.tsx Show resolved Hide resolved

src/components/MarkdownCard/MarkdownCard.tsx Outdated Show resolved Hide resolved

src/components/Heading/Heading.tsx Outdated Show resolved Hide resolved

allanpope reviewed

View reviewed changes

src/components/ContentCards/ContentCards.tsx Show resolved Hide resolved

johnniedfeldt and others added 8 commits

July 19, 2022 11:26


          Update src/components/ContentCards/ContentCards.tsx

Co-authored-by: Allan Pope <[email protected]>


          Add Heading size prop

8ee6594


          Merge branch 'master' into QUIC-1590-widget-markdown-card


          Reduced version of react-markdown for compatibility reasons and to fi…

ddc459e

…x jest test case issues.


          Fix test issue.

f2cfb3f


          Added remark-gfm back in for Github Markdown support.

d3ea33f


          Added more test cases.

3c62a08


          PR Comment resolutions.

e064603

github-advanced-security bot found potential problems

View reviewed changes

src/components/MarkdownCard/MarkdownCard.test.tsx Fixed Show fixed Hide fixed

src/components/MarkdownCard/MarkdownCard.test.tsx Fixed Show fixed Hide fixed


          Ran prettier and added prettier check to husky.

b9b6a60

allanpope reviewed

View reviewed changes

.husky/pre-commit Outdated Show resolved Hide resolved

johnniedfeldt added 2 commits

July 21, 2022 14:43


          More PR changes

b1581c5


          Remove unused import

c249c88

allanpope reviewed

View reviewed changes

src/components/Heading/Heading.tsx Outdated Show resolved Hide resolved


          Adjusted Table and code styling. Change Heading size implementation.

d7030f3

johnniedfeldt and others added 2 commits

July 21, 2022 20:02


          Merge branch 'master' into QUIC-1590-widget-markdown-card

fe58d77


          Update src/components/Heading/Heading.tsx

9c7de3c

Co-authored-by: Allan Pope <[email protected]>

github-advanced-security bot found potential problems

View reviewed changes

src/components/MarkdownCard/MarkdownCard.tsx Fixed Show fixed Hide fixed


          Fixed lint issues. Still have 3 warnings in MarkdownCard.tsx

1a21387

johnniedfeldt marked this pull request as ready for review

July 22, 2022 03:25

allanpope reviewed

View reviewed changes

src/components/MarkdownCard/MarkdownCard.tsx Outdated Show resolved Hide resolved

Contributor

allanpope commented Jul 22, 2022

Could you add a cypress test https://github.com/soulmachines/sm-web-component/blob/master/cypress/e2e/content-cards.spec.ts#L7

It tests the full flow, loads a browser with the snippet and enters the command in the input and verifies that a card shows. In your case you'd add the command from the QA test corpus and then just expect a certain bit of text to show up

allanpope reviewed

View reviewed changes

src/components/Heading/Heading.tsx Show resolved Hide resolved


          Added markdown test. Markdown block does not show for some reason, bu…

ffe1b31

…t can run cypress tests.

allanpope reviewed

View reviewed changes

src/components/MarkdownCard/MarkdownCard.tsx Outdated Show resolved Hide resolved

johnniedfeldt added 3 commits

July 25, 2022 14:44


          possible debug updates for package.json

dec0179


          react-markdown updated to lates. Cypress now works for Markdown card.

13bc6d6


          Jest fix

079d819

github-advanced-security bot found potential problems

View reviewed changes

src/components/MarkdownCard/MarkdownCard.test.tsx Fixed Show fixed Hide fixed


          Minor PR fixes

6bc5468

Contributor Author

johnniedfeldt commented Jul 27, 2022

@allanpope I've fixed the Cypress issues by updating react-markdown and remark-gfm and creating a mock for each. Now the Jest tests pass but there are a lot of tests we cannot currently run. For now I have commented them out since I figure we will add them back in later when the Jest issue gets resolved. Is this fine or would you like me to remove them completely? Also, I'll see about removing the warnings that causes.

allanpope reviewed

View reviewed changes

jest.config.js Outdated Show resolved Hide resolved

__mocks__/react-markdown.tsx Outdated Show resolved Hide resolved

src/components/MarkdownCard/MarkdownCard.test.tsx Outdated Show resolved Hide resolved

allanpope reviewed

View reviewed changes

cypress/e2e/content-cards.spec.ts Show resolved Hide resolved

johnniedfeldt added 4 commits

July 28, 2022 11:00


          PR comment implementations

edea7be


          Remove unused test

c7487f3


          Fix unused variable warnings

d8ca1cf


          Remove extra unused variable

f5e255f

johnniedfeldt requested a review from allanpope

July 28, 2022 19:18

allanpope reviewed

View reviewed changes

__mocks__/react-markdown.tsx Outdated Show resolved Hide resolved

cypress/e2e/content-cards.spec.ts Outdated Show resolved Hide resolved


          Minor fixes

7b7bc7f

Contributor

allanpope commented Jul 28, 2022

Looks good, you've just go a merge conflict to fix


          Merge branch 'master' into QUIC-1590-widget-markdown-card

ccd539d

allanpope approved these changes

View reviewed changes

allanpope merged commit 4778907 into master

allanpope deleted the QUIC-1590-widget-markdown-card branch

July 28, 2022 23:00

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet